Skip to content

[fix](paimon) Preserve statement schemas and timestamp precision - #67904

Merged
Gabriel39 merged 13 commits into
apache:masterfrom
Gabriel39:fix/paimon-schema-and-timestamp
Sep 15, 2026
Merged

Gabriel39 merged 13 commits into
apache:masterfrom
Gabriel39:fix/paimon-schema-and-timestamp

Conversation

@Gabriel39

@Gabriel39 Gabriel39 commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Paimon schema-only ALTER operations can advance the schema without creating a data snapshot. Reusing a data fence as a schema selector can rewind the analyzed schema, and timestamp predicate conversion can lose fractional precision and prune matching rows.

This PR keeps a statement's Paimon schema independent of its data fence. A single immutable statement pin carries the schema and physical schema/snapshot coordinates, including fallback branch-local data coordinates. Metadata reloads and INSERT replanning either reuse that generation or reject a changed physical table. Reader options retain catalog and relation override precedence and replay compatibility while preserving authentication and table decorators. Retained table objects are checked against their own physical schema version before restoring a pin or deriving option overrides. NTZ predicates preserve microseconds; comparisons requiring nanosecond precision remain Doris residual predicates.

Scope and compatibility

Latest-schema publication in the generic FE bridge is an explicit connector opt-in, enabled by Paimon. A positive schema ID alone does not change ordinary schema publication. Iceberg and Hive retain their existing behavior. Existing explicit time-travel cache policies and historical Iceberg partition-spec isolation are outside this PR.

The typed opt-in changes the connector plugin contract, so its API version advances from 7.0 to 8.0. Connector plugins must be rebuilt for the matching API; the public surface baseline and compatibility tests cover this contract.

Release note

Fix Paimon schema consistency after schema-only changes and preserve timestamp predicate precision. Validate retained statement generations across metadata reloads and fallback branch recreation.

Test

  • Latest Paimon module validation: 598 tests, zero failures/errors, one existing connectivity test skipped; plugin packaging verified with install.
  • Four new tests fail before these fixes and pass afterward: unknown/malformed reader options during connector replay, and table recreation before the latest schema read with equal/different schema IDs. Replay tests reconstruct the connector twice and read through both scan and backend-table preparation.
  • Existing tests continue to cover schema-only ALTER, option precedence, authentication, delegated tables and fallback branches.
  • FE/SPI integration validation on 31945b1: 928 related tests and the full 142-test SPI suite, FE build, and all 10 Native/JNI result checks in test_paimon_schema_only_snapshot_precision passed. The current follow-up changes only the Paimon module; the regression output was generated by the harness.
  • Standalone Checkstyle passed across the complete FE reactor with skipping and the Maven build cache explicitly disabled.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/34735774547

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes: four findings remain (three P1, one P2). The NTZ constructor change itself is sound, but the schema/data-generation handling is not end-to-end safe.

Critical checkpoint conclusions:

  • Goal and proof: The goal is to retain the current Paimon schema while fencing data and to preserve NTZ sub-millisecond predicates. M1, M2, and M4 show the schema/MVCC goal is not achieved in branch, cached-table, and schema-derived system paths; M3 shows the end-to-end precision regression does not exercise NTZ.
  • Scope and focus: The patch is otherwise focused across three production files and targeted tests. No additional user focus was supplied, so the full changed-file scope was reviewed.
  • Concurrency: No threads or locks are added. External data/schema commits can race statement binding and planning; the unfenced branch transition is M1 and exact-schema drift is M2.
  • Lifecycle and static initialization: Table handles, transient Paimon tables, and scan-option copies are statement-scoped; no new static initialization or resource-release issue was found. The branch handle reload lifecycle is part of M1.
  • Configuration: No production configuration is added. The regression restores force_jni_scanner in finally.
  • Compatibility: No public SPI, thrift, storage format, symbol, or rolling-upgrade surface changes. Paimon 1.3.1 provides the APIs used.
  • Parallel paths: Normal, branch, options, native, JNI, system-table, privilege, fallback, partition, and statistics paths were traced. Distinct gaps are M1, M2, and M4.
  • Special conditions: The new FileStoreTable/preserve-marker condition is documented but insufficient: it excludes schema-derived wrappers (M4), and the marker carries no exact schema generation (M2).
  • Test coverage: Added unit cases cover a fresh ordinary table and the converter helper, but miss branch mutation, warm catalog cache, schema-derived system aliases, and a true NTZ end-to-end setup (M1-M4). No additional negative/empty-branch test covers the first-commit race.
  • Test results: Expected rows are deterministic for the stable setup, but the precision assertions can pass via residual filtering because the Spark column is LTZ (M3). No result file is changed.
  • Observability: No new logging or metrics are required for this local read-path change; no separate observability defect was found.
  • Transactions and persistence: No Doris transaction, EditLog, failover, or persisted-metadata path is modified.
  • Data writes and atomicity: Production changes are read-only; no write/crash atomicity path is modified.
  • FE/BE variables: No new cross-process variable or thrift field is introduced. Existing serialized-table/native paths are inconsistent only as described in M2/M4.
  • Performance: The added map/table copies are bounded per statement; no distinct CPU, memory, or asymptotic regression was found.
  • Other issues: No additional correctness, error-handling, security, or style blocker remained after three converged review rounds.

Static review only: per the review contract, no builds or tests were run, so author/CI test claims were not independently executed.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16965 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 4ae6c48c3265cce9a18f85cb99ecac6ff86b1067, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17587	3116	3113	3113
q2	2098	265	256	256
q3	10185	873	519	519
q4	4668	253	199	199
q5	7671	574	383	383
q6	138	118	96	96
q7	547	496	390	390
q8	9239	897	894	894
q9	3437	2366	2381	2366
q10	6513	873	697	697
q11	387	199	181	181
q12	610	255	200	200
q13	18143	1532	1160	1160
q14	162	146	137	137
q15	q16	450	393	368	368
q17	1402	839	767	767
q18	3099	2229	2233	2229
q19	1265	872	783	783
q20	378	290	202	202
q21	5621	1790	1845	1790
q22	339	274	235	235
Total cold run time: 93939 ms
Total hot run time: 16965 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3486	3440	3449	3440
q2	503	395	365	365
q3	2171	2535	2114	2114
q4	1191	1189	895	895
q5	2176	2124	2080	2080
q6	165	119	89	89
q7	1004	919	865	865
q8	1588	1404	1418	1404
q9	3150	3121	3137	3121
q10	1886	1786	1628	1628
q11	352	267	256	256
q12	455	429	349	349
q13	1481	1532	1134	1134
q14	169	181	174	174
q15	q16	398	389	354	354
q17	3570	3341	3228	3228
q18	4797	4444	4723	4444
q19	858	927	855	855
q20	1008	951	837	837
q21	3811	3054	3190	3054
q22	402	344	319	319
Total cold run time: 34621 ms
Total hot run time: 31005 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 82255 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 4ae6c48c3265cce9a18f85cb99ecac6ff86b1067, data reload: false

query5	4246	429	337	337
query6	378	140	123	123
query7	4936	428	226	226
query8	289	128	132	128
query9	8679	2892	2889	2889
query10	404	223	186	186
query11	5406	1051	914	914
query12	126	72	71	71
query13	1196	454	319	319
query14	6109	2214	2093	2093
query14_1	1977	1982	1959	1959
query15	178	114	114	114
query16	908	359	348	348
query17	783	456	357	357
query18	2328	334	238	238
query19	166	141	107	107
query20	72	72	71	71
query21	198	102	86	86
query22	5514	5414	5479	5414
query23	6737	6227	6106	6106
query23_1	5997	6053	5986	5986
query24	7281	1116	763	763
query24_1	748	791	776	776
query25	429	299	262	262
query26	1216	231	130	130
query27	2795	423	252	252
query28	4689	1505	1478	1478
query29	922	430	354	354
query30	257	158	129	129
query31	823	399	341	341
query32	129	74	78	74
query33	460	232	183	183
query34	971	819	475	475
query35	399	400	344	344
query36	558	574	533	533
query37	118	78	74	74
query38	996	846	812	812
query39	500	478	484	478
query39_1	469	469	465	465
query40	200	91	80	80
query41	59	60	54	54
query42	74	73	73	73
query43	242	244	219	219
query44	997	529	540	529
query45	112	112	101	101
query46	764	856	554	554
query47	757	764	706	706
query48	312	317	240	240
query49	542	249	191	191
query50	722	261	191	191
query51	8135	8201	8215	8201
query52	67	66	71	66
query53	200	204	141	141
query54	224	267	166	166
query55	70	61	57	57
query56	195	160	170	160
query57	692	658	632	632
query58	186	158	156	156
query59	1273	1251	1104	1104
query60	231	187	168	168
query61	126	136	110	110
query62	385	217	172	172
query63	172	138	137	137
query64	2868	653	654	653
query65	1679	1566	1585	1566
query66	1892	256	203	203
query67	9751	12006	9599	9599
query68	2940	1242	745	745
query69	344	217	203	203
query70	678	657	577	577
query71	268	172	160	160
query72	2210	1648	1652	1648
query73	633	558	343	343
query74	1970	1201	1134	1134
query75	1152	1079	963	963
query76	2382	701	494	494
query77	252	256	217	217
query78	3943	3686	3228	3228
query79	2379	833	569	569
query80	1552	313	267	267
query81	484	151	134	134
query82	597	133	92	92
query83	268	201	187	187
query84	290	113	88	88
query85	764	346	273	273
query86	384	169	161	161
query87	1020	948	915	915
query88	2764	2130	2119	2119
query89	273	197	177	177
query90	1993	120	128	120
query91	127	115	94	94
query92	78	68	65	65
query93	1435	1038	700	700
query94	647	263	226	226
query95	512	317	225	225
query96	815	575	270	270
query97	1046	1069	1035	1035
query98	167	132	130	130
query99	414	356	313	313
Total cold run time: 177531 ms
Total hot run time: 82255 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.79 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 4ae6c48c3265cce9a18f85cb99ecac6ff86b1067, data reload: false

query1	0.01	0.00	0.01
query2	0.07	0.04	0.03
query3	0.24	0.10	0.10
query4	1.60	0.10	0.09
query5	0.18	0.16	0.16
query6	1.27	0.69	0.69
query7	0.03	0.01	0.01
query8	0.05	0.03	0.03
query9	0.28	0.21	0.22
query10	0.35	0.34	0.34
query11	0.16	0.12	0.12
query12	0.15	0.12	0.12
query13	0.31	0.33	0.30
query14	0.46	0.45	0.45
query15	0.36	0.35	0.34
query16	0.23	0.24	0.23
query17	0.63	0.69	0.70
query18	0.19	0.17	0.17
query19	1.19	1.19	1.16
query20	0.01	0.01	0.01
query21	15.42	0.15	0.12
query22	5.09	0.04	0.04
query23	16.18	0.25	0.10
query24	2.99	0.31	0.26
query25	0.10	0.05	0.04
query26	0.84	0.17	0.12
query27	0.03	0.03	0.03
query28	3.62	0.58	0.28
query29	12.46	3.16	2.59
query30	0.24	0.11	0.12
query31	2.75	0.37	0.17
query32	3.55	0.32	0.22
query33	1.50	1.41	1.47
query34	15.35	2.16	1.81
query35	1.74	1.74	1.73
query36	0.44	0.29	0.28
query37	0.06	0.04	0.04
query38	0.04	0.03	0.02
query39	0.04	0.02	0.02
query40	0.12	0.08	0.08
query41	0.07	0.03	0.02
query42	0.03	0.02	0.03
query43	0.04	0.03	0.03
Total cold run time: 90.47 s
Total hot run time: 14.79 s

### What problem does this PR solve?

Related PR: apache#67904

Problem Summary: Cached Paimon tables and system wrappers could use a different
schema than statement binding, and branch reads could observe later commits.
Carry the exact schema generation and positive or empty branch data fence
through binding, native planning and JNI serialization. Use explicit NTZ
literals in the timestamp precision regression.

### Release note

Paimon statement reads retain consistent schema and data generations.

### Check List (For Author)

- Test: Paimon package build (570 passed, 1 skipped); FE MVCC unit tests
  (68 passed); FE Checkstyle. External regression updated, not run locally.
- Behavior changed: Yes, schema and data fences remain consistent through reads.
- Does this need documentation: No.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of exact head 4ffdc1fc1d5b4dbf94ad6dac896b9c24a9976bb3 against base 73b2a153cb5eb34b474386d678ee4a02af5788bd. I am requesting changes for four P1 correctness defects and one P2 regression-standard defect, detailed inline.

Checkpoint conclusions:

  • Goal and proof: the ordinary stale-table, system-wrapper, branch-fence, and microsecond cases are addressed, but schema-owned options, fallback topology, TIMESTAMP(7..9), and Iceberg's generic latest pin remain inconsistent; the release-note claim is therefore not yet established for supported paths.
  • Focus and scope: all 12 changed files plus relevant callers and consumers were reviewed; no user-specific focus was supplied. The change is otherwise focused.
  • Concurrency and lifecycle: the schema memo is statement-scoped, concurrently safe, and discarded with metadata; handle identity separates base/system/branch tables. No lock-order or teardown issue found.
  • Configuration and compatibility: no new user configuration, persisted format, public symbol, storage protocol, or FE-BE field is introduced. The internal marker is stripped before Paimon/backend options.
  • Parallel paths and conditions: plain, OPTIONS, branch, empty, system-wrapper, fallback/decorator, native dictionary, catalog-loader removal, JNI serialization, and Iceberg paths were traced. The surviving defects are the five inline points; prior GitHub threads were treated as duplicate fences.
  • Tests and results: the changed unit tests statically reach the main intended paths but miss renamed schema-owned options, stale fallback pairs, precision above 6, and mixed Iceberg cache generations. Stable regression rows also bypass the required qt_/order_qt_ generated-output workflow. This review ran no builds or tests. The author reports 570 Paimon tests, 68 MVCC tests, and Checkstyle passing; the external Spark/Doris regression was not run locally.
  • Observability and performance: existing errors/logging are adequate, the per-statement map and schema lookup are bounded, and no separate observability or performance blocker was found.
  • Transactions, persistence, and data writes are unchanged. Memory-safety and BE nullable checkpoints are not applicable to this FE Java/test-only diff.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 17215 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 4ffdc1fc1d5b4dbf94ad6dac896b9c24a9976bb3, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17717	3061	3065	3061
q2	2093	264	227	227
q3	10234	849	527	527
q4	4668	253	208	208
q5	7663	575	393	393
q6	140	113	100	100
q7	541	507	394	394
q8	9233	931	938	931
q9	3538	2407	2420	2407
q10	6493	866	721	721
q11	390	201	182	182
q12	615	263	194	194
q13	18164	1545	1183	1183
q14	161	157	139	139
q15	q16	450	401	380	380
q17	1312	894	810	810
q18	3198	2316	2270	2270
q19	1257	939	806	806
q20	375	294	204	204
q21	5604	1840	1897	1840
q22	328	273	238	238
Total cold run time: 94174 ms
Total hot run time: 17215 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3417	3374	3390	3374
q2	502	394	377	377
q3	2356	2867	2302	2302
q4	1220	1206	916	916
q5	2261	2165	2151	2151
q6	172	123	91	91
q7	1041	945	861	861
q8	1606	1410	1409	1409
q9	3204	3169	3178	3169
q10	1910	1855	1675	1675
q11	362	270	254	254
q12	462	440	344	344
q13	1496	1548	1189	1189
q14	192	177	162	162
q15	q16	399	409	370	370
q17	3699	3417	3313	3313
q18	4976	4538	5118	4538
q19	906	858	842	842
q20	1040	992	842	842
q21	3839	3141	3222	3141
q22	400	344	308	308
Total cold run time: 35460 ms
Total hot run time: 31628 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 83453 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 4ffdc1fc1d5b4dbf94ad6dac896b9c24a9976bb3, data reload: false

query5	4269	417	342	342
query6	393	148	128	128
query7	4938	401	233	233
query8	309	138	138	138
query9	8710	2936	2909	2909
query10	415	249	193	193
query11	5387	1057	924	924
query12	117	75	80	75
query13	1215	456	341	341
query14	6121	2249	2137	2137
query14_1	2019	2019	1980	1980
query15	175	125	113	113
query16	921	370	377	370
query17	783	451	372	372
query18	2349	322	242	242
query19	163	135	115	115
query20	77	74	72	72
query21	205	102	90	90
query22	5510	5479	5437	5437
query23	6961	6396	6298	6298
query23_1	6197	6285	6100	6100
query24	7255	1107	780	780
query24_1	797	782	775	775
query25	429	308	253	253
query26	1223	248	132	132
query27	2773	422	265	265
query28	4680	1507	1498	1498
query29	905	414	335	335
query30	249	157	124	124
query31	827	396	331	331
query32	114	74	79	74
query33	458	226	175	175
query34	981	816	485	485
query35	404	418	346	346
query36	585	558	547	547
query37	117	86	73	73
query38	1007	874	845	845
query39	504	513	487	487
query39_1	483	503	465	465
query40	204	89	76	76
query41	55	53	51	51
query42	72	72	72	72
query43	241	245	213	213
query44	993	539	549	539
query45	110	111	101	101
query46	807	842	524	524
query47	806	770	715	715
query48	310	303	227	227
query49	541	246	200	200
query50	745	260	199	199
query51	8348	8146	8158	8146
query52	71	68	60	60
query53	201	205	147	147
query54	222	160	165	160
query55	77	65	56	56
query56	200	165	160	160
query57	709	692	669	669
query58	212	156	169	156
query59	1237	1233	1095	1095
query60	257	187	167	167
query61	135	117	113	113
query62	354	210	180	180
query63	170	143	155	143
query64	2739	693	665	665
query65	1665	1664	1646	1646
query66	1914	287	258	258
query67	10183	9821	9943	9821
query68	2800	1219	756	756
query69	343	225	202	202
query70	679	628	586	586
query71	252	181	178	178
query72	2250	1689	1546	1546
query73	679	562	329	329
query74	1574	1229	1186	1186
query75	1194	1120	977	977
query76	2297	723	535	535
query77	256	266	211	211
query78	4043	3683	3299	3299
query79	2812	838	569	569
query80	1584	325	281	281
query81	500	160	135	135
query82	620	124	103	103
query83	274	217	189	189
query84	294	111	91	91
query85	780	352	289	289
query86	477	177	181	177
query87	1037	1010	913	913
query88	3489	2120	2137	2120
query89	280	196	175	175
query90	2115	136	132	132
query91	129	121	96	96
query92	98	73	74	73
query93	3196	1140	691	691
query94	637	253	237	237
query95	539	331	236	236
query96	795	608	274	274
query97	1086	1084	1030	1030
query98	186	141	135	135
query99	416	346	312	312
Total cold run time: 181767 ms
Total hot run time: 83453 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.97 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 4ffdc1fc1d5b4dbf94ad6dac896b9c24a9976bb3, data reload: false

query1	0.00	0.00	0.01
query2	0.08	0.04	0.04
query3	0.25	0.12	0.11
query4	1.60	0.10	0.11
query5	0.18	0.16	0.16
query6	1.25	0.70	0.67
query7	0.04	0.01	0.01
query8	0.05	0.03	0.03
query9	0.28	0.22	0.22
query10	0.35	0.35	0.36
query11	0.17	0.12	0.12
query12	0.15	0.12	0.13
query13	0.32	0.32	0.32
query14	0.47	0.46	0.45
query15	0.38	0.36	0.36
query16	0.21	0.24	0.21
query17	0.74	0.69	0.70
query18	0.19	0.18	0.19
query19	1.21	1.16	1.22
query20	0.02	0.01	0.01
query21	15.44	0.17	0.11
query22	5.06	0.04	0.05
query23	16.18	0.25	0.11
query24	3.00	0.33	0.28
query25	0.12	0.04	0.04
query26	0.83	0.17	0.12
query27	0.04	0.02	0.02
query28	3.72	0.55	0.26
query29	12.46	3.20	2.58
query30	0.26	0.12	0.12
query31	2.76	0.39	0.18
query32	3.50	0.33	0.22
query33	1.45	1.50	1.53
query34	15.35	2.24	1.83
query35	1.77	1.81	1.74
query36	0.46	0.30	0.28
query37	0.06	0.04	0.04
query38	0.04	0.03	0.03
query39	0.04	0.02	0.02
query40	0.12	0.07	0.08
query41	0.08	0.03	0.02
query42	0.04	0.03	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.76 s
Total hot run time: 14.97 s

### What problem does this PR solve?

Related PR: apache#67904

Problem Summary: Restoring a bound Paimon schema could retain field-referencing options from a different generation or overwrite a fallback branch's schema and identity. Exact nanosecond timestamp pushdown could reject values that compare equal after Doris truncation. Iceberg cached schema pins could expose live partition names and specs. Preserve each schema's options and fallback provenance, retain privilege checks, keep high-precision comparisons residual, and pin Iceberg partition specs with their schema.

### Release note

External scans retain consistent schema and partition metadata. Paimon nanosecond timestamps are compared at Doris precision without unsafe source pruning.

### Check List (For Author)

- Test: 1,963 connector unit tests passed, 6 skipped; one existing Iceberg test failure reproduced before the changes and excluded. FE Checkstyle and plugin packages passed. Generated and verified all five regression baselines through the regression harness.
- Behavior changed: Yes, preserve schema provenance and avoid false timestamp pruning.
- Does this need documentation: No.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39 Gabriel39 changed the title [fix](paimon) Preserve current schema and timestamp precision [fix](fe) Preserve external schema generations and timestamp precision Sep 13, 2026
@Gabriel39

Copy link
Copy Markdown
Contributor Author

Addressed the five review findings in 75ceac1:

  • Preserve bound Paimon schema options and replay only catalog/runtime overrides; added a bucket/sequence-key warm-cache rename test.
  • Restore the main fallback leaf independently, retain the fallback schema/branch, and preserve the privilege checker; verified fallback-only rows and SELECT denial enforcement.
  • Leave TIMESTAMP(7..9) comparisons residual; covered nanosecond values across comparison operators.
  • Pin Iceberg partition spec IDs with schema IDs and resolve source names against the pinned schema; covered a shared latest-cache hit after live rename/spec evolution.
  • Converted all five fixed result checks to order_qt_ cases, generated the .out with the regression harness, and reran successfully.

Validation: 1,963 connector tests passed and 6 skipped; one pre-existing Iceberg MERGE test failure was reproduced on the previous PR head and excluded. FE Checkstyle, FE build, and plugin packages passed. Local regression used the current FE/JNI extension with an existing BE and disabled the FE local-shuffle planner only on that isolated test instance for compatibility.

The PR description has been updated. GitHub review-reply and thread-resolution APIs are returning server errors, so this top-level comment records the fixes while those discussion updates remain unavailable.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16922 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 75ceac12b856aca84f572e2337b52870837950b7, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17572	3077	3068	3068
q2	2097	264	223	223
q3	10226	924	535	535
q4	4669	252	209	209
q5	7672	561	402	402
q6	137	119	96	96
q7	533	499	392	392
q8	9228	895	930	895
q9	3530	2439	2436	2436
q10	6522	868	708	708
q11	403	206	179	179
q12	612	263	205	205
q13	18126	1559	1180	1180
q14	160	147	139	139
q15	q16	427	398	365	365
q17	1362	824	756	756
q18	3182	2274	2307	2274
q19	1264	892	786	786
q20	385	279	209	209
q21	5591	1628	1885	1628
q22	341	271	237	237
Total cold run time: 94039 ms
Total hot run time: 16922 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3433	3384	3379	3379
q2	495	399	375	375
q3	2252	2347	2170	2170
q4	1206	1179	917	917
q5	2203	2169	2126	2126
q6	166	120	90	90
q7	1042	968	882	882
q8	1600	1406	1405	1405
q9	3171	3172	3137	3137
q10	1905	1855	1677	1677
q11	355	272	250	250
q12	453	428	349	349
q13	1467	1533	1184	1184
q14	172	185	156	156
q15	q16	395	401	363	363
q17	3619	3291	3249	3249
q18	4900	4510	4788	4510
q19	998	877	875	875
q20	1012	977	862	862
q21	3869	3217	3189	3189
q22	400	354	320	320
Total cold run time: 35113 ms
Total hot run time: 31465 ms

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review outcome: request changes on exact head 75ceac12b856aca84f572e2337b52870837950b7.

Goal and focus: this PR separates data, schema, and Iceberg spec generations for statement-stable external-table reads and fixes Paimon timestamp literal precision. No additional review focus was supplied. The implementation addresses the previously reported branch, wrapper, bound-schema, schema-builder, timestamp-test, and regression-baseline gaps, but three statement-consistency blockers remain.

Blocking findings:

  1. Paimon exact-schema discovery/materialization/restoration performs schema-file I/O after the connector authentication/TCCL scope has closed.
  2. Iceberg records the latest spec ID for schema assembly but drops it before partition-view construction and cache identity.
  3. The generic MVCC layer computes an exact local pinned schema, then builds partition items through ambient no-argument schema accessors before the pin is registered.

Checkpoint conclusions:

  • Goal alignment and tests: the changed tests exercise warm-table Paimon schema restoration, branch/options/native/JNI paths, NTZ precision, Iceberg schema assembly, and generated regression output. They stop before the three failing boundaries described inline.
  • Scope, clarity, and reuse: the changes are focused and reuse the statement snapshot abstraction, but the coordinate is not propagated through every generic/connector consumer.
  • Concurrency and synchronization: statement maps and immutable/single-flight caches show no new lock-order or mutation race; omitting specId from the derived cache identity is nevertheless a cross-generation correctness error.
  • Lifecycle and cleanup: statement metadata ownership and connector close paths are sound. Snapshot registration timing leaves partition construction unable to observe the local pin.
  • Configuration: no new configuration contract is introduced. Existing metadata-cache behavior and secured connector deployments expose the reported gaps.
  • Compatibility and parallel paths: no persisted or FE/BE wire format changes were found. Base tables, OPTIONS/system/fallback/branch variants, empty snapshots, native/JNI reads, and RANGE/LIST Iceberg paths were checked; existing comments remain duplicate fences.
  • Conditions and error handling: timestamp comparison, IN/OR, null propagation, and residual filtering are consistent. The remaining schema I/O can escape authentication, while mixed partition arity/types can fail analysis or silently omit partitions.
  • Test execution: per the review contract, I did not build or run tests; conclusions are static. Changed tests were inspected, including the generated ordered regression baseline.
  • Observability: no separate logging or metrics gap was identified; fixing context and generation propagation is preferable to diagnosing the resulting failures later.
  • Persistence, transactions, and writes: not applicable beyond read-side metadata coordinates; no EditLog, storage-format, or data-write change is present.
  • Resource and null safety: Java-only changes introduce no BE ownership or memory-safety issue; nullable timestamp/filter paths retain residual evaluation.
  • Performance: statement memoization and bounded caches are reasonable. The spec-view cache key must include the complete generation coordinate.
  • Security and other risks: the Doris threat model was applied. Privilege-wrapper retention is sound and no security vulnerability is asserted; the Paimon issue is authenticated connector correctness/availability in secured deployments.

The three inline P1 findings must be fixed and covered before approval.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 83385 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 75ceac12b856aca84f572e2337b52870837950b7, data reload: false

query5	4243	415	342	342
query6	403	134	124	124
query7	4928	415	227	227
query8	289	132	117	117
query9	8658	2917	2932	2917
query10	399	225	203	203
query11	5392	1053	962	962
query12	114	74	73	73
query13	1193	458	315	315
query14	6157	2252	2130	2130
query14_1	2021	2005	2017	2005
query15	177	124	125	124
query16	932	408	380	380
query17	801	478	364	364
query18	2334	324	241	241
query19	167	147	121	121
query20	84	74	73	73
query21	201	103	89	89
query22	5566	5520	5420	5420
query23	6853	6335	6164	6164
query23_1	6323	6259	6249	6249
query24	7236	1106	774	774
query24_1	747	792	777	777
query25	427	303	253	253
query26	1239	238	127	127
query27	2778	428	256	256
query28	4670	1516	1507	1507
query29	925	453	358	358
query30	244	159	133	133
query31	821	417	342	342
query32	137	75	78	75
query33	458	232	175	175
query34	988	840	494	494
query35	411	404	357	357
query36	589	580	520	520
query37	120	81	72	72
query38	1021	860	851	851
query39	521	497	479	479
query39_1	497	452	480	452
query40	203	96	79	79
query41	60	58	58	58
query42	77	74	76	74
query43	255	246	218	218
query44	991	526	562	526
query45	123	113	105	105
query46	793	869	549	549
query47	775	748	719	719
query48	310	291	241	241
query49	540	245	245	245
query50	756	259	197	197
query51	8158	8165	8083	8083
query52	73	73	59	59
query53	213	233	150	150
query54	207	171	157	157
query55	73	62	63	62
query56	203	160	197	160
query57	726	657	662	657
query58	196	175	176	175
query59	1228	1245	1128	1128
query60	239	179	183	179
query61	112	106	107	106
query62	345	201	177	177
query63	173	144	146	144
query64	2699	725	604	604
query65	1620	1665	1657	1657
query66	1774	269	208	208
query67	9990	10042	9952	9952
query68	2985	1209	690	690
query69	339	233	209	209
query70	677	632	619	619
query71	250	172	162	162
query72	2316	1757	1369	1369
query73	682	591	329	329
query74	2007	1249	1171	1171
query75	1201	1135	983	983
query76	2356	725	537	537
query77	257	271	213	213
query78	4073	3822	3292	3292
query79	2363	813	595	595
query80	1600	340	277	277
query81	493	157	137	137
query82	625	129	97	97
query83	273	212	198	198
query84	290	111	90	90
query85	788	342	282	282
query86	392	173	180	173
query87	1039	970	908	908
query88	2771	2144	2129	2129
query89	289	199	180	180
query90	1998	132	129	129
query91	131	127	98	98
query92	81	71	76	71
query93	1431	1102	708	708
query94	631	229	237	229
query95	537	251	228	228
query96	791	557	281	281
query97	1068	1108	1063	1063
query98	144	140	135	135
query99	420	345	318	318
Total cold run time: 178826 ms
Total hot run time: 83385 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 15.27 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 75ceac12b856aca84f572e2337b52870837950b7, data reload: false

query1	0.01	0.00	0.01
query2	0.08	0.03	0.04
query3	0.24	0.12	0.11
query4	1.60	0.10	0.10
query5	0.19	0.16	0.16
query6	1.25	0.72	0.69
query7	0.03	0.01	0.00
query8	0.05	0.04	0.03
query9	0.29	0.21	0.22
query10	0.36	0.37	0.36
query11	0.18	0.12	0.12
query12	0.16	0.12	0.12
query13	0.31	0.31	0.31
query14	0.47	0.47	0.46
query15	0.38	0.35	0.36
query16	0.24	0.23	0.24
query17	0.68	0.71	0.70
query18	0.18	0.17	0.17
query19	1.24	1.20	1.20
query20	0.02	0.01	0.01
query21	15.45	0.16	0.14
query22	5.08	0.04	0.05
query23	16.17	0.26	0.11
query24	2.96	0.31	0.27
query25	0.11	0.04	0.04
query26	0.82	0.17	0.13
query27	0.03	0.04	0.03
query28	3.62	0.58	0.31
query29	12.43	3.18	2.59
query30	0.26	0.11	0.12
query31	2.75	0.38	0.17
query32	3.51	0.32	0.24
query33	1.42	1.65	1.63
query34	15.36	2.28	1.80
query35	1.81	1.80	1.78
query36	0.46	0.29	0.28
query37	0.06	0.04	0.04
query38	0.04	0.03	0.03
query39	0.04	0.02	0.02
query40	0.11	0.08	0.08
query41	0.07	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.59 s
Total hot run time: 15.27 s

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@924060929 924060929 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current head with the local pipeline-style review. The previously reported schema, timestamp, authentication, and Iceberg identity fixes are present, but four distinct Major regressions remain: captured-schema lifetime across INSERT replanning, fallback-child schema restoration, catalog option provenance, and HMS Iceberg capability forwarding. Details are inline.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes. Static review of exact head 54a5294b6cb8490693a157d0d465a47e5e15c9a6 found two additional P1 correctness/availability failures. An equivalent P2 catalog-option provenance issue was posted meanwhile in discussion r4004077352, so I have not duplicated it.

Critical checkpoints

  • Goal and proof: The PR aims to keep external-table data, schema, spec, and predicate semantics coherent across schema-only evolution and recreation. The direct paths are substantially covered, but the goal is incomplete: Paimon can cross physical generations within one statement, and valid UUID-less Iceberg evolution is rejected.
  • Focus/minimality: The 19-file diff is focused on Paimon/Iceberg MVCC pinning, generic FE consumers, and their tests; no unrelated edits were found.
  • Concurrency/thread safety: External catalog ALTER/commit/DROP/CREATE operations are the relevant concurrent actors. Cache values are immutable and statement maps use the existing concurrent machinery; no new lock-order or deadlock issue was found. The Paimon logical handle key nevertheless lacks a physical-generation fence, producing the first inline issue.
  • Lifecycle: Statement metadata, independently resolved Paimon handles, Iceberg statement-frozen tables, cache invalidation/retry, leases, authentication scopes, and decorator teardown were traced. The two inline issues are lifecycle/cache-boundary defects; no separate leak, double close, circular lifetime, or static-initialization issue was found.
  • Configuration: No new public configuration is added. Existing no-cache/reload Paimon behavior makes the generation crossing reachable; REST vended credentials intentionally keep latestSnapshotCache while disabling tableCache, making the Iceberg failure reachable. Existing paimon.table-option.* precedence has a separate equal-value provenance gap already covered by r4004077352.
  • Compatibility: No persisted format, SPI symbol, EditLog, or FE-BE wire change is introduced. UUID-less format-V1 Iceberg metadata is an explicitly supported compatibility path, and the new exact-filename identity fallback breaks valid same-table commits there. No additional rolling-upgrade issue was found.
  • Parallel paths: Latest, empty, reader-only OPTIONS, explicit selectors, branches, supported system wrappers, RANGE/LIST/UNPARTITIONED materialization, synchronous/streaming/count scans, native dictionaries, JNI serialization, and metadata tables were checked. No additional distinct issue survived; the known explicit Paimon historical-memo and Iceberg partition-view/spec points are already fenced by existing discussions.
  • Conditions and error handling: The Iceberg mismatch error includes the table and retry guidance, but its condition is over-broad for ordinary UUID-less commits. Paimon numeric-ID/equality checks cannot prove physical identity. No separate swallowed status/exception or speculative defensive branch was found.
  • Test coverage: The added unit/regression tests broadly cover cache hits, empty-to-first-append, recreation, schema rename, partition arity/type, synthesized versus generated columns, authentication, fallback/branch behavior, native/JNI paths, and timestamp precision. They miss recreation between one statement's pin and later scan-handle resolution, and an ordinary UUID-less M1-to-M2 commit under warm-pin/fresh-table caching.
  • Test results/output: Query labels and expected regression output were checked statically. Per the review contract, I did not run builds or tests; author/CI-reported results are not independent verification.
  • Observability: Existing errors carry useful table/retry context. Paimon generation mixing has no fail-loud signal, while the Iceberg signal fires on valid evolution; fixing those conditions is the actionable need. No separate logging or metrics gap was substantiated.
  • Transactions/persistence: The changed code is read-metadata/planning logic and adds no Doris transaction, journal, failover, or persistent-state behavior.
  • Data writes: No Doris data-write path is modified. Existing Iceberg writer identity ancestry was inspected as evidence for valid V1 evolution; no separate write atomicity issue was found.
  • FE-BE variables: No new transmitted variable or scattered protocol send path is added; snapshot identity/schema/spec properties remain FE-internal. Native/JNI consumers were included in the consistency trace.
  • Performance, memory, and BE nullability: This is FE Java code; no BE allocator/nullability issue applies. Schema/identity work is bounded to statement/cache operations. Apart from the recurring valid-query retry and the already-reported lost reader policy, no material CPU or memory regression was found.
  • Other data correctness: Paimon timestamp precision 0-6 preserves the visible wall clock; precision 7-9 pushdown fails closed while Doris retains residual evaluation. The synthesized-column exemption remains narrow enough that physical/generated skew fails loudly. No further distinct issue was found.

User focus: No additional user-provided focus was supplied; the full authoritative diff was reviewed.

Completion: Complete after three convergence rounds. All candidates were accepted, dismissed with code evidence, or duplicate-fenced; this review submits the two remaining distinct findings.

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/34917962088

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16649 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 499a96e8c1171abf80d30fbc76d407b2a5a7d96a, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17583	2993	2982	2982
q2	2058	251	220	220
q3	10265	853	514	514
q4	4662	247	203	203
q5	7677	538	381	381
q6	136	114	94	94
q7	560	489	425	425
q8	9246	961	871	871
q9	3361	2348	2345	2345
q10	6474	874	698	698
q11	398	201	181	181
q12	609	254	196	196
q13	18145	1508	1120	1120
q14	164	146	145	145
q15	q16	429	393	366	366
q17	1337	922	752	752
q18	3004	2224	2235	2224
q19	1288	844	782	782
q20	383	279	204	204
q21	5671	1722	1811	1722
q22	325	270	224	224
Total cold run time: 93775 ms
Total hot run time: 16649 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3369	3288	3297	3288
q2	512	389	370	370
q3	2152	2300	2136	2136
q4	1164	1142	876	876
q5	2122	2072	2071	2071
q6	171	117	85	85
q7	994	933	835	835
q8	1556	1360	1360	1360
q9	3064	3021	2994	2994
q10	1847	1787	1595	1595
q11	349	259	244	244
q12	458	420	337	337
q13	1480	1538	1143	1143
q14	163	158	146	146
q15	q16	384	386	356	356
q17	3518	3243	3194	3194
q18	4754	4445	4675	4445
q19	851	814	789	789
q20	942	1104	825	825
q21	3770	3069	3288	3069
q22	370	336	321	321
Total cold run time: 33990 ms
Total hot run time: 30479 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 81073 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 499a96e8c1171abf80d30fbc76d407b2a5a7d96a, data reload: false

query5	4232	409	329	329
query6	393	137	122	122
query7	4946	402	224	224
query8	281	129	124	124
query9	8699	2858	2867	2858
query10	434	225	190	190
query11	5367	1030	902	902
query12	113	69	71	69
query13	1196	446	314	314
query14	6023	2204	2055	2055
query14_1	1906	1933	1944	1933
query15	176	118	110	110
query16	922	382	350	350
query17	1045	448	363	363
query18	2547	331	232	232
query19	163	140	112	112
query20	81	71	69	69
query21	202	102	85	85
query22	5407	5420	5329	5329
query23	6771	6290	6156	6156
query23_1	6149	6130	6193	6130
query24	7264	1077	767	767
query24_1	776	789	753	753
query25	419	292	248	248
query26	1232	229	129	129
query27	2776	408	248	248
query28	4668	1511	1462	1462
query29	923	442	352	352
query30	247	152	131	131
query31	812	395	325	325
query32	133	77	72	72
query33	467	228	186	186
query34	988	796	480	480
query35	411	403	345	345
query36	564	545	509	509
query37	123	78	72	72
query38	995	835	816	816
query39	483	475	478	475
query39_1	457	472	452	452
query40	198	95	77	77
query41	58	55	54	54
query42	75	72	73	72
query43	241	239	208	208
query44	983	545	539	539
query45	121	108	133	108
query46	806	861	515	515
query47	741	741	709	709
query48	302	310	213	213
query49	527	233	183	183
query50	744	254	197	197
query51	8082	7766	7905	7766
query52	66	72	58	58
query53	196	192	144	144
query54	207	159	156	156
query55	71	58	55	55
query56	184	167	160	160
query57	797	654	647	647
query58	183	160	161	160
query59	1181	1202	1088	1088
query60	222	177	177	177
query61	131	135	120	120
query62	336	205	193	193
query63	165	140	140	140
query64	2683	699	561	561
query65	1649	1537	1627	1537
query66	1876	264	221	221
query67	9487	9522	9678	9522
query68	3003	1198	735	735
query69	340	223	200	200
query70	708	609	605	605
query71	253	170	159	159
query72	2257	1665	1477	1477
query73	651	553	333	333
query74	2008	1220	1110	1110
query75	1158	1079	951	951
query76	2357	709	518	518
query77	253	263	196	196
query78	3990	3663	3140	3140
query79	2329	832	574	574
query80	1590	328	263	263
query81	488	152	132	132
query82	619	120	96	96
query83	269	203	189	189
query84	243	108	89	89
query85	758	327	276	276
query86	395	171	168	168
query87	1008	961	883	883
query88	2773	2063	2115	2063
query89	274	194	172	172
query90	2006	127	127	127
query91	129	113	96	96
query92	70	64	69	64
query93	1406	1099	677	677
query94	641	257	226	226
query95	506	265	311	265
query96	796	576	272	272
query97	1038	1086	991	991
query98	142	137	132	132
query99	415	343	308	308
Total cold run time: 177394 ms
Total hot run time: 81073 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.5 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 499a96e8c1171abf80d30fbc76d407b2a5a7d96a, data reload: false

query1	0.00	0.01	0.01
query2	0.08	0.04	0.04
query3	0.25	0.11	0.10
query4	1.60	0.10	0.10
query5	0.17	0.16	0.16
query6	1.26	0.68	0.69
query7	0.03	0.01	0.00
query8	0.04	0.03	0.03
query9	0.28	0.21	0.21
query10	0.34	0.33	0.34
query11	0.16	0.11	0.12
query12	0.14	0.12	0.12
query13	0.30	0.31	0.30
query14	0.44	0.45	0.45
query15	0.36	0.35	0.34
query16	0.23	0.20	0.23
query17	0.71	0.67	0.72
query18	0.18	0.16	0.16
query19	1.21	1.14	1.14
query20	0.01	0.01	0.01
query21	15.46	0.16	0.11
query22	5.08	0.05	0.04
query23	16.17	0.27	0.10
query24	2.98	0.32	0.24
query25	0.11	0.04	0.03
query26	0.82	0.17	0.12
query27	0.03	0.02	0.03
query28	3.62	0.56	0.25
query29	12.51	3.15	2.55
query30	0.25	0.12	0.11
query31	2.76	0.37	0.17
query32	3.53	0.31	0.22
query33	1.33	1.36	1.47
query34	15.55	2.19	1.78
query35	1.72	1.73	1.75
query36	0.44	0.29	0.29
query37	0.06	0.04	0.03
query38	0.04	0.03	0.03
query39	0.04	0.02	0.02
query40	0.11	0.07	0.08
query41	0.07	0.02	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.54 s
Total hot run time: 14.5 s

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes after a complete static review of exact head 499a96e8c1171abf80d30fbc76d407b2a5a7d96a and all 22 changed files. The bounded review converged in round 3: every candidate is resolved, and the final set is two P1 consistency defects plus one P2 regression-oracle gap, detailed inline.

Critical-checkpoint conclusions:

  • Goal and proof: The patch largely retains latest Paimon/Iceberg schemas with their data coordinates through metadata and scan consumers, but the goal is incomplete: the Paimon fallback child's data generation is not fenced, and Iceberg query-schema DDL can combine a pinned schema with a live sort order. The later reader-option behavior also lacks repository-standard generated regression output.
  • Scope and focus: The implementation is otherwise focused on connector schema capture/validation, reconstruction, generic MVCC consumers, and timestamp predicate safety. I reviewed the complete authoritative diff and relevant callers/consumers. The user-focus file supplied no additional focus beyond the full PR.
  • Concurrency: The accepted failures are reproducible with sequential operations; no race is required. Statement maps use immutable values/concurrent containers and connector caches retain their existing synchronization. I found no new lock-order, blocking-under-lock, unsafe-publication, or deadlock defect.
  • Lifecycle/static initialization: Statement-owned pins and borrowed/tracked tables have bounded lifetimes and existing teardown. No circular ownership, resource leak, or static-initialization issue was found.
  • Configuration: No configuration item is added. Existing positive Iceberg latest-cache TTL plus the REST-vended fresh-table path is the concrete trigger for the Iceberg issue; configuration parsing/dynamic-update behavior is unchanged.
  • Compatibility/persistence: No public SPI, function symbol, thrift/protocol field, EditLog/image record, or storage format changes. I found no separate rolling-upgrade, replay, or persisted-state issue.
  • Parallel paths and conditions: I checked latest/empty/time-travel pins, Paimon OPTIONS/branches/system/fallback/privilege/native/JNI paths, Iceberg direct/HMS/UUID-less/partition/handle/write paths, Hive/Hudi generic behavior, and /api/query_schema. Identity mismatches otherwise fail loud; the two uncovered consistency arms are the inline P1s.
  • Tests and results: Changed unit/regression tests cover most schema evolution, recreation, authentication/TCCL, option precedence, partition, and generated/synthesized-column paths. Missing cases are fallback-only same-schema recreation, Iceberg sort evolution through query-schema DDL, and generated .out entries for the later five deterministic reader-option checks. The existing five .out labels match their queries. Per this review assignment, I ran no build or test, so execution claims are not independently verified.
  • Observability/error handling: Generation mismatch errors include retry guidance, and no additional metric is warranted for statement-scoped planning. The Iceberg defect silently emits inconsistent DDL, so it needs a correctness fix and regression rather than logging alone. No other error-propagation issue survived review.
  • Transactions, persistence, and data writes: These changes are read/planning-side; they add no transaction, journal, or data-write atomicity path.
  • FE/BE variables: No new session/system variable or thrift field crosses FE/BE. Existing connector snapshot properties/table options carry the bound state through reconstruction.
  • Performance: Added schema/digest work is bounded per statement/table and memoized where designed. I found no unbounded loop, per-row work, or distinct hot-path regression.
  • Other correctness: Paimon timestamp precision 0-6 remains exactly pushable and precision above 6 remains residual across comparison operators. No additional distinct correctness issue remained after duplicate fencing and final missed-area sweeps.

Review status: complete static review at the exact live head; changes requested for the three inline issues.

### What problem does this PR solve?

Related PR: apache#67904

Problem Summary: Latest-schema retention was implicitly enabled by a schema ID,
changing ordinary metadata publication for unrelated connectors. Make retention
an explicit Paimon opt-in and remove the collateral Iceberg/Hive changes. Keep
schema and physical coordinates in one immutable statement pin, including the
fallback branch data fence, so reloads cannot silently select replacement data.
Preserve upstream snapshot-aware partition listing when integrating master.

### Release note

Preserve Paimon statement schemas and fallback data coordinates. The connector
plugin API advances to 8.0 for explicit latest-schema publication.

### Check List (For Author)

- Test: Related FE/connector suites, full SPI suite, FE build, standalone FE
  Checkstyle, and Native/JNI regression queries.
- Behavior changed: Yes; only explicitly opted-in latest pins publish retained
  schemas, and changed fallback generations are rejected.
- Does this need documentation: No; existing Paimon query semantics are restored.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39 Gabriel39 changed the title [fix](fe) Preserve external schema generations and timestamp precision [fix](paimon) Preserve statement schemas and timestamp precision Sep 15, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review at exact head 31945b11ac196c3c31041cac0e09d13ba00d8390.

Status: capped/incomplete. The final allowed convergence round materially broadened the second finding from the equal-ID shortcut to both restoration branches, so the contract permits no fourth round.

Two distinct P1 issues remain; see the inline comments.

Checkpoints: the Paimon-only, default-false retainSchema scope and connector API-major/surface updates are consistent; non-Paimon latest behavior, storage/EditLog/thrift formats, and runtime configuration are unchanged. Statement pin map ownership, lifecycle, authentication/TCCL, branch/fallback/system routing, NTZ residual handling, and bounded metadata-read cost were traced without another distinct issue. Existing raw/rendered discussions were applied as a hard duplicate fence.

Static inspection only as required: no build, unit test, or regression test was run, and author/CI test claims were not independently executed.

### What problem does this PR solve?

Related PR: apache#67904

Problem Summary: Schema restoration strictly reparsed properties accepted by
catalog replay, making ordinary scans fail on ignored legacy reader options.
It could also combine a replacement table's live schema with a retained old
table object after DROP/CREATE. Preserve compatible option parsing and validate
the retained schema against its own persisted version before either restoration
branch, excluding legitimate dynamic option overlays from that comparison.

### Release note

Keep replayed Paimon catalogs queryable and reject stale table objects during
statement schema restoration.

### Check List (For Author)

- Test: Four failing regressions reproduced before the fix; 598 Paimon tests
  (one existing connectivity test skipped), plugin packaging, and FE Checkstyle.
- Behavior changed: Yes; restored scans retain replay compatibility and reject
  retained objects from a different physical schema generation.
- Does this need documentation: No; preserves existing catalog/query contracts.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review conclusion: I found no new actionable issue on the current head after treating all existing inline threads and raw review comments as hard duplicate fences. I would approve this head based on the static evidence reviewed here.

Critical checkpoint conclusions:

  • Goal and proof: the change now binds Paimon's latest schema independently of its data snapshot, carries physical schema/snapshot coordinates through the statement, validates reusable IDs before scan reconstruction, and preserves that schema through FE materialization and native/JNI planning. The added FE/SPI tests and external regression cover schema-only ALTER, empty/first-append and DROP/CREATE races, fallback branches, system/privilege wrappers, option precedence, and timestamp precision. Test execution reported in the PR/threads is author/CI evidence; I did not run builds or tests under this review contract.
  • Focus and smallness: although the diff is necessarily cross-layer, each public SPI/FE change is tied to the Paimon opt-in. The connector-local digest/restoration helpers keep generation validation and decorator reconstruction centralized; I found no unrelated production behavior change.
  • Concurrency: the relevant concurrency is external Paimon ALTER/commit/DROP/CREATE racing FE binding and planning. statementPins is a per-metadata ConcurrentHashMap, its immutable values are computed once per logical base/system/branch handle, and expensive catalog/file reads occur outside Doris locks. No new lock ordering or deadlock surface is introduced; generation changes fail closed at restoration.
  • Lifecycle: one metadata instance is memoized for one statement, so its pins expire with the statement; INSERT scope replacement rehydrates from carried coordinates. Catalog-wide snapshot/schema/partition caches retain their pre-existing lifecycle. No static-initialization, circular-reference, or resource-release issue was found.
  • Configuration: no new configuration key is added. Existing Paimon catalog and relation reader options retain relation-over-catalog-over-physical precedence, replay filters incompatible historical values, and validation occurs at the effective planning boundary.
  • Compatibility: ConnectorMvccSnapshot.retainSchema defaults false, only Paimon latest pins opt in, the frozen SPI surface is updated, and the connector API major is bumped to 8 so incompatible external plugins fail closed. No storage format, EditLog, or new Thrift field is introduced.
  • Parallel paths: I traced plain latest reads, selector-free OPTIONS, explicit snapshot/tag/time, incremental and branch reads, empty tables, system tables, partition materialization, native planning, JNI/backend serialization, and transient-table reload. Other connectors keep the default-off schema-retention behavior.
  • Conditions and errors: the new retain-schema, empty-fence, wrapper, synthesized-column, and timestamp-precision branches are narrowly gated and documented. Physical/generated schema misses and generation/digest mismatches remain fail-loud with retry context; unsupported decorators fail loudly rather than dropping policy.
  • Data correctness: schema and data fences are validated before the executable scan, fallback children retain independent coordinates, and sub-microsecond source timestamps remain residual while precisions up to microseconds preserve the full literal. No changed-line wrong-row path survived the final trace.
  • Test coverage and results: changed tests include positive and negative unit cases plus ordered regression queries for plain/options/branch and native/JNI behavior. The checked .out labels align with the query labels and deterministic ordering requirements. I did not independently regenerate the result file.
  • Observability: the retry error identifies a changed Paimon table generation and existing planning/authentication logs retain table context. This statement-local correctness fence does not require a new long-lived metric.
  • Persistence, writes, and crash behavior: this is a read-planning/SPI change; it adds no Doris transaction, data-write, EditLog, or persistent-state path, so master failover and write atomicity are not implicated.
  • FE/BE variables: no new FE-to-BE protocol field is added. The already-serialized Paimon table and schema-evolution dictionary are rebuilt from the same bound source for native and JNI paths.
  • Performance and memory: the opted-in path adds one eager mapped-schema materialization per table/statement; statement pins and existing catalog caches bound repeated work. Digest maps are statement-scoped, and no unbounded collection, hot-loop I/O, BE allocator, nullable-column, or obvious asymptotic regression was found.
  • Other issues and review focus: no additional user focus was supplied. A final missed-area sweep of all 22 changed files, existing comments, and the shared risk ledger found no distinct issue left to post.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 81.25% (13/16) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 10.16% (13/128) 🎉
Increment coverage report
Complete coverage report

@Gabriel39
Gabriel39 merged commit 0aea714 into apache:master Sep 15, 2026
47 checks passed
@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Sep 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants